-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
es_out: support Upstream Servers with configuration overriding #7608
base: master
Are you sure you want to change the base?
es_out: support Upstream Servers with configuration overriding #7608
Conversation
812020d
to
ba3382a
Compare
|
||
struct flb_elasticsearch { | ||
struct flb_elasticsearch_config { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The whole idea / implementation and naming of new types follows Upstream support in Forward output plugin.
return 0; | ||
} | ||
|
||
toks = flb_utils_split((const char *)cloud_auth, ':', -1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not:
toks = flb_utils_split((const char *)cloud_auth, ':', -1); | |
toks = flb_utils_split((const char *) cloud_auth, ':', -1); |
because https://github.com/fluent/fluent-bit/blob/master/CONTRIBUTING.md requires to follow https://httpd.apache.org/dev/styleguide.html which requires to not put a space in case of type cast:
The Guidelines
...
There is no whitespace between a cast and the item modified (e.g.,"(int)j"
and not"(int) j"
).
Hi reviewers, Is it possible to approve only workflow for this pull request, so that automated checks and build can start? Thank you. |
@mabrarov sure |
Hi @PettitWesley, It looks like all failed checks are around run-macos-unit-tests jobs and caused by the following failed unit tests:
I feel like other pull requests have the same issues, i.e. it doesn't seem that the failed checks are caused by this pull request changes. Help of maintainers is appreciated. Thank you. |
plugins/out_es/es_conf.c
Outdated
} | ||
tmp = flb_upstream_node_get_property(FLB_ES_CONFIG_PROPERTY_LOGSTASH_PREFIX, node); | ||
if (tmp) { | ||
ec->logstash_prefix = (char *)tmp; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Type cast is illegal. Should use flb_sds_create
instead. Working on fix in feature/out_es_upstream_support_extended_test branch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
ba3382a
to
b7cd81b
Compare
Hi @PettitWesley, Is it possible to trigger automated workflow (build) for this pull request one more time? I found & fixed one issue and added tests for the new code since last build happened. Thank you. |
Hi dear reviewers, Is it possible to get this pull request reviewed / accepted sooner? Is there something pending / waiting from my side to start review? Thank you. |
b7cd81b
to
b81d3f7
Compare
Hi @PettitWesley and @edsiper, It feels like you are code owners for Elasticsearch output plugin. Is there something pending / waiting from my side to start review of this pull request? This new feature was requested 4 years ago and I feel it is something which multiple users of Fluent Bit (not just my team) would like to have. Thank you. |
b81d3f7
to
f6431c2
Compare
Hi @PettitWesley and @edsiper, Is there a chance to get this pull request reviewed (and eventually accepted)? Please let me know if something is missing and blocking that. Thank you. |
Please someone review this, I also want this feature to merged and released. |
f6431c2
to
d03cde0
Compare
+1 Hello reviewers, could you please review this PR? This feature also will be very useful in our project and very useful feature to support HA. Thank you! |
Just FYI, my team successfully tested replacement of following log shipping solution: Fluent Bit DaemonSet in K8s (Forward output plugin with Upstream Servers, Upstream node per Fluentd instance) → Fluentd (2 instances with fluent-plugin-elasticsearch configured to connect to all nodes of Elasticsearch cluster) → Elasticsearch cluster (3+ nodes) with: Fluent Bit DaemonSet in K8s (Docker image built from source branch of this pull request, Elasticsearch output plugin with Upstream Servers, Upstream node per Elasticsearch node) → Elasticsearch cluster (3+ nodes) For my team it means (this pull request means) a possibility to get rid of 7-9 Fluentd instances (where each Fluentd instance occupies a single VM). |
This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days. |
This pull request is still actual and up-to-date (ready for merging, tested with changes coming from target branch). |
Hello, @PettitWesley and @edsiper. i hope you guys can take a look at this pr and merge it sooner, it's a highly requested feature with its mentioned in the official docs as well. |
Hi @PettitWesley and @edsiper, It looks like there is one more enhancement which this pull request brings comparing to approach "Fluent Bit (Forward output plugin with Upstream Servers) → multiple Fluentd instances → multiple Elasticsearch instances of a single Elasticsearch cluster":
|
d03cde0
to
30a2822
Compare
Hi @PettitWesley, Could you (or someone from maintainers / reviewers) please approve workflow for this pull request, so that the automated checks and build of this pull request can start? Thank you. |
7aaea9f
to
b13a6f8
Compare
b13a6f8
to
f7d0a15
Compare
…wn to parser of Upstream node configuration section are implemented, e.g. "host" and "port" Signed-off-by: Marat Abrarov <[email protected]>
…o the test callback based on configuration of Fluent Bit and based on configuration of plugin Signed-off-by: Marat Abrarov <[email protected]>
…llback contract Signed-off-by: Marat Abrarov <[email protected]>
…with Upstream node configuration For Elastic cloud authentication these parameters are always taken from plugin configuration and never from Upstream node configuration: cloud_id. For AWS authentication these parameters are always taken from plugin configuration and never from Upstream node configuration: http_proxy, no_proxy, tls*. Signed-off-by: Marat Abrarov <[email protected]>
Signed-off-by: Marat Abrarov <[email protected]>
f7d0a15
to
f023976
Compare
Implementation of Upstream feature for the Elasticsearch output plugin.
This pull request is based on pull request #1560 and Forward output plugin.
It was tested in a local setup with:
Single Elasticsearch instance.
Refer to elastic-cluster directory of mabrarov/elastic-stack repository for Docker Compose project used to create target Elasticsearch cluster and Kibana.
fluent-bit.conf Fluent Bit configuration file used for the test:
Debug log is available at flb_es.log.
Elasticsearch cluster of 3 master-eligible/data and 1 coordinating nodes.
Refer to elastic-cluster directory of mabrarov/elastic-stack repository for Docker Compose project used to create target Elasticsearch cluster and Kibana.
fluent-bit.conf Fluent Bit configuration file used for the test:
upstream.conf Fluent Bit configuration file used for the test:
Debug log is available at flb_es_upstream.log.
Testing
ok-package-test
label to test for all targets (requires maintainer to do).Documentation
Backporting
Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.